fix(openapi): stop the seam erasing path parameters on any route with a body (#9705) - #9739
Conversation
… a body (#9705) registerRouteSpec built one object literal carrying the key `request` twice: path parameters in the first, body and query in a conditional spread further down. A property arriving through a later spread replaces an earlier one of the same name, and TypeScript does not flag it because the duplicate is a spread rather than a literal duplicate key. So any route declaring BOTH a templated segment and a request body or query published no `parameters` array at all -- exactly what pathParameters() exists to prevent, since a templated segment with no matching parameter is a schema-validation warning and leaves a generated client holding a URL it cannot fill. Latent only because every production entry today is response-only. The first migrated POST /v1/repos/:owner/:repo/... would have dropped owner and repo silently, and the existing test registered precisely that combination while asserting only that requestBody was defined. The fix is to build `request` once; the emitter, pathParameters(), and toSpecPath() are untouched. The regenerated openapi.json is byte-identical, as expected while no caller passes a request. Four cases pin it -- body, query, both, and the unchanged no-request arm -- and each of the first three fails against the pre-fix emitter. The existing spec-only assertion now also checks its `id` parameter, and a new case covers the ORB and webhook auth levels, whose own security schemes had no test.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 8b72a90 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 07:31 AM |
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 07:48:18 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
loopover-orb
Bot
added
the
gittensor:bug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9739 +/- ##
==========================================
- Coverage 90.28% 89.46% -0.82%
==========================================
Files 907 907
Lines 113356 113318 -38
Branches 26894 26891 -3
==========================================
- Hits 102342 101383 -959
- Misses 9683 10846 +1163
+ Partials 1331 1089 -242
Flags with carried forward coverage won't be shown. Click here to find out more.
|
1 participant
Closes #9705.
The defect
registerRouteSpecbuilt one object literal and handed it toregistry.registerPath. That literal contained the keyrequesttwice — once where path parameters were added, and again inside a conditional spread that fired when a body or query was declared. A property arriving through a later spread replaces an earlier one of the same name, and TypeScript does not flag it, because the duplicate arrives via a spread rather than as a literal duplicate key.So any route declaring both a templated path segment and a request body or query published no
parametersarray at all. That is precisely the failurepathParameters()'s own doc comment says it exists to prevent: a templated segment with no matching parameter is a schema-validation warning and leaves a generated client holding a URL it cannot fill.It was latent only because no production caller passes
requestyet — every entry inorb-and-control-route-specs.tsandinternal-and-public-route-specs.tsis response-only. The first migratedPOST /v1/repos/:owner/:repo/...handler, the stated next step of #9531, would have lostownerandrepofrom the published document silently.The existing test registered exactly the broken combination (
/v1/spec-only/:idplus a body) and asserted onlyexpect(operation?.requestBody).toBeDefined(), so the defect passed the suite.The fix
Build
requestonce, merging path parameters, body, and query into a single key. The singleregistry.registerPathcall,pathParameters(), andtoSpecPath()are untouched — the fix is to stop emitting the key twice, not to restructure the emitter.Fails-before / passes-after
Verified by stashing only
src/openapi/define-route.tsand re-running the suite. Against the pre-fix emitter:After the fix, 15 passed. The fourth new case — a route with path params and no request at all — passes both ways by design: it is the arm that must be unchanged, and it is there so a regression in the other direction cannot hide.
Also in this PR
The ORB and webhook auth levels in
securityForhad no test. They are the two levels that are not a LoopOver credential at all — an ORB-issued instance token, and a webhook verified by a body signature carrying no bearer — so publishing the generic bearer/cookie pair for either would tell a client to send something the route does not accept. Now asserted.Validation
apps/loopover-ui/public/openapi.jsonregenerated: byte-identical, as the issue predicted while no caller passes a request.ui:openapi:checkpasses.npx vitest run --changed=origin/main: 1913 passed, 142 files.tsc --noEmitclean.src/openapi/define-route.tspatch is comment-and-deletion only — no new executable lines — and every arm of the merged conditional (path params present/absent × body present/absent × query present/absent) is exercised by the four new/extended cases.